fix(api): always use node:https.request in apiFetch to avoid undici b…#1319
Merged
Conversation
…ody timeout Node's built-in fetch uses undici which has a default bodyTimeout of 300s. For large streaming ND-JSON responses (e.g. full scan results) this timeout fires before the body has fully transferred, producing a BodyTimeoutError. Route all apiFetch calls through the existing _httpsRequestFetch path, which uses node:https.request and has no body timeout — consistent with the SDK and all other HTTP clients used across socket-cli, socket-sdk-js, and coana. When SSL_CERT_FILE is not set, agent is passed as undefined and https.request uses its default agent. Behaviour is otherwise identical to before. Update the apiFetch test to assert https.request is always used and that no custom HttpsAgent is created when CA certs are not configured.
There was a problem hiding this comment.
Pull request overview
This PR updates the CLI’s low-level API request helper to always use node:https.request instead of Node’s built-in fetch (undici), avoiding undici’s default 300s bodyTimeout that can interrupt long-running / large responses.
Changes:
- Route all
apiFetchcalls through the existinghttps.requestimplementation path. - Allow
_httpsRequestFetchto accept anHttpsAgent | undefinedso the default agent is used when no extra CA certs are configured. - Update unit tests to assert
https.requestis always used and that no custom agent is created when CA certs are not configured.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/utils/api.mts |
Removes fetch fallback and always uses _httpsRequestFetch with an optional agent. |
src/utils/api.test.mts |
Updates tests to reflect always-https.request behavior and validates agent handling when CA certs are absent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ering Resolving the promise as soon as headers arrive and constructing the Response with a ReadableStream body, matching fetch() semantics. Previously the response body was fully buffered before the promise resolved, which broke streaming call sites such as streamDownloadWithFetch that pipe response.body to disk expecting a live stream.
Bret Comnes (bcomnes)
force-pushed
the
fix-inconsistant-timeouts
branch
from
271d6e3 to
837cb45
Compare
John-David Dalton (jdalton)
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ody timeout
Node's built-in fetch uses undici which has a default bodyTimeout of 300s. For large streaming ND-JSON responses (e.g. full scan results) this timeout fires before the body has fully transferred, producing a BodyTimeoutError.
nodejs/undici
v6.21.3 —lib/dispatcher/client.js`, line ~246Route all apiFetch calls through the existing _httpsRequestFetch path, which uses node:https.request and has no body timeout — consistent with the SDK and all other HTTP clients used across socket-cli, socket-sdk-js, and coana.
When SSL_CERT_FILE is not set, agent is passed as undefined and https.request uses its default agent. Behaviour is otherwise identical to before.
Update the apiFetch test to assert https.request is always used and that no custom HttpsAgent is created when CA certs are not configured.
Note
Medium Risk
All CLI API traffic now routes through the custom
https.requestimplementation, which could subtly change request/redirect behavior compared tofetchand affect all outbound calls. Mitigated by reusing the existing_httpsRequestFetchpath and tightening tests around agent handling.Overview
apiFetchno longer falls back to globalfetch; it now always uses the existing_httpsRequestFetch(node:https.request) path so large/streaming ND-JSON responses aren’t subject to undici’s default body timeout.When no extra CA certs are configured, the request is made with
agent: undefined(default agent); whenSSL_CERT_FILEprovides extra CAs, a customHttpsAgentis still created and passed. Tests were updated to asserthttps.requestis always used and that the agent behavior matches both scenarios.Reviewed by Cursor Bugbot for commit ee86ba4. Configure here.